Skip to content

Conversation

@pecoram
Copy link
Contributor

@pecoram pecoram commented Jan 24, 2025

In Chrome versions before 67, data: URLs were treated as uncertain-origin resources for security reasons. In this context:

XMLHttpRequest did not allow access to base64 (data:) URLs because it violated CORS (Cross-Origin Resource Sharing) policies. Even though a data: URL is "local," it was still considered a different origin from the domain where the request was made.

@pecoram pecoram changed the title fix: replaced XMLHttpRequest with fetch fix: Image Worker - replaced XMLHttpRequest with fetch Jan 24, 2025
@pecoram pecoram changed the title fix: Image Worker - replaced XMLHttpRequest with fetch fix(Image Worker): replaced XMLHttpRequest with fetch Jan 24, 2025
@wouterlucas
Copy link
Contributor

I think we should skip the image worker entirely if we know its a data: url - no point to push this over to a child process boundary just to decode source?

@pecoram
Copy link
Contributor Author

pecoram commented Jan 24, 2025

I agree! What do others think?

in that case should we leave XMLHttpRequest? on the ImageTexture we use fetch

@wouterlucas
Copy link
Contributor

There's already a fetch function if you skip the image worker, it will automatically fallback to fetch which should work out of the box:
https://github.com/lightning-js/renderer/blob/main/src/core/textures/ImageTexture.ts#L208-L223

as far as I can see extending that if condition with a startsWith should do it?

@pecoram pecoram changed the title fix(Image Worker): replaced XMLHttpRequest with fetch fix: Exclude Base64 images from service worker handling Jan 24, 2025
@pecoram pecoram changed the title fix: Exclude Base64 images from service worker handling fix: Exclude Base64 images from image worker handling Jan 24, 2025
@pecoram pecoram requested a review from wouterlucas January 27, 2025 10:29
Copy link
Contributor

@wouterlucas wouterlucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks

@wouterlucas wouterlucas added this pull request to the merge queue Jan 27, 2025
Merged via the queue into lightning-js:main with commit 6acce40 Jan 27, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants